-
Notifications
You must be signed in to change notification settings - Fork 694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce install-chaos-faults duration #4796
base: master
Are you sure you want to change the base?
Reduce install-chaos-faults duration #4796
Conversation
Signed-off-by: Jemin <[email protected]>
Signed-off-by: Jemin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
args: [ | ||
'kubectl apply -f /tmp/ -n {{workflow.parameters.adminModeNamespace}} && faultCount=$(ls /tmp/ | grep -E "(.yaml$|.yml$)" | wc -l) && until [[ $(kubectl get -f /tmp/ --no-headers | wc -l) -eq $faultCount ]]; do sleep 1; echo "Waiting for ChaosExperiment CR Ready..." ; done; echo "ChaosExperiment CR Ready"' | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jemlog
There are some edge cases which can happen here.
Lets say older CR for chaos experiments already exists on user's cluster. Now with this command, it will always pass & won't actually wait for new CR to get applied. Which is fine incase we were not making any changes to experiment CR. But with every release, we update go-runner in chaos-experiment CR. So, it's required that only when new experiment CR is applied on cluster, then only we should move on to other step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Jonsy13
To solve the edge case you mentioned, I included the process of deleting the existing ChaosExperiment CR first.
I tried this solution because CR generation does not take much time. Please tell me if there are other edge cases.
Thanks!
Signed-off-by: Jemin <[email protected]>
…//github.com/jemlog/litmus into feature/update-chaosexperimentCR-create-time
can you check a build-pipeline @jemlog ? |
Signed-off-by: Jemin <[email protected]>
…//github.com/jemlog/litmus into feature/update-chaosexperimentCR-create-time
Proposed changes
Previously, even though all ChaosExperiment CRs were created, the delay time of install-chaos-faults was long because it slept for an additional 30 seconds.
To solve this problem, I counted the number of yaml files under the /tmp directory and assigned it to
faultCount
variable. Next, I added a loop operation to check whether the number of CRs searched throughkubectl get -f /tmp/ --no-headers | wc -l
is same asfaultCount
.This operation can shorten the install-chaos-fault execution time.
Types of changes
What types of changes does your code introduce to Litmus? Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Dependency
Special notes for your reviewer: